Skip to content

fix(ts): export types explicitly - #14428

Merged
Pavel Feldman (pavelfeldman) merged 1 commit into
microsoft:mainfrom
pavelfeldman:export-types
May 26, 2022
Merged

fix(ts): export types explicitly#14428
Pavel Feldman (pavelfeldman) merged 1 commit into
microsoft:mainfrom
pavelfeldman:export-types

Conversation

@pavelfeldman

@pavelfeldman Pavel Feldman (pavelfeldman) commented May 26, 2022

Copy link
Copy Markdown
Member

Fixes #14348

yury-s
Yury Semikhatsky (yury-s) approved these changes May 26, 2022
View reviewed changes

@yury-s Yury Semikhatsky (yury-s) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test for this.

@pavelfeldman

Pavel Feldman (pavelfeldman) commented May 26, 2022

Copy link
Copy Markdown
Member Author

I won't need a test once we move to TS 4.7. We have the opposite problem though, we might need bots for earlier versions of TS.

@pavelfeldman
Pavel Feldman (pavelfeldman) merged commit 6efb1ec into microsoft:main May 26, 2022
Pavel Feldman (pavelfeldman) added a commit that referenced this pull request May 26, 2022
@pavelfeldman
cherry-pick(#14428): fix(ts): export types explicitly
b9e9f4d
@andipaetzold Andi Pätzold (andipaetzold) mentioned this pull request May 27, 2022
Closed
@PaperStrike 华 (PaperStrike) mentioned this pull request Jun 4, 2022
Closed
@AviVahl

Avi Vahl (AviVahl) commented Jun 21, 2022 •
edited
Loading

Copy link
Copy Markdown

any chance for a bugfix release with this change? just noticed it didn't resolve types in native esm mode (node16 module/moduleResolution)... 😮

was going to report it using the following reproduction steps:

mkdir playwright-node16-bug
cd playwright-node16-bug
npm init -y
npm i playwright-core typescript -D
npx tsc --init --module node16 --moduleResolution node16
echo "import playwright from 'playwright-core'; console.log(playwright);" > bug.mts
npx tsc

@mxschmitt

Max Schmitt (mxschmitt) commented Jun 21, 2022

Copy link
Copy Markdown
Contributor

Avi Vahl (@AviVahl) you can use the next version npm install playwright-core@next!

@AviVahl

Avi Vahl (AviVahl) commented Jun 21, 2022

Copy link
Copy Markdown

thanks for the tip. 👍 would be useful in own projects, but my team is using some dependencies with stable playwright peerDeps.

I'm currently using "module": "esnext", "moduleResolution": "node", but wanted to switch to "node16", as it actually works as I'd expect (picks up "exports").

Jonathan Pollak (TxHawks) added a commit to TxHawks/react-polymorphic-types that referenced this pull request Jun 26, 2022
@TxHawks
fix: missing types with "moduleResolution":"Node16"
a5c3594 
Since the filename pointed at by `default` and `import` (`empty`) is not identical to the filename of the `d.ts` file (`index`), the top level `types` field is not enough when using `"moduleResolution":"Node16"` and TypeScript `4.7`. 

To fix this, `types` must be explicitly specified inside the `exports` object in addition to the top level `types` field.

See [firebase/firebase-js-sdk#6300](/firebase/firebase-js-sdk/issues/6300) and [microsoft/playwright#14428](/microsoft/playwright/pull/14428)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yury-s Yury Semikhatsky (yury-s) yury-s approved these changes
+1 more reviewer
@rwoll Ross Wollman (rwoll) rwoll approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Types not compatible with TS4.7 Node ESM support

5 participants

@pavelfeldman @AviVahl @mxschmitt @yury-s @rwoll